Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bzlmod): Fixing Windows Python Interpreter symlink issues #1265

Merged
merged 2 commits into from
Jun 13, 2023

Conversation

chrislovecnm
Copy link
Collaborator

@chrislovecnm chrislovecnm commented Jun 13, 2023

When using Windows you cannot run the symlink directly.

Because of how symlinks work in Windows we need to path realpath resolve the link.
Unlike Linux and OSX we cannot execute the Python symlink directly. Windows throws an error "-1073741515", when we execute the symlink directory. This error means that the Python binary cannot find dlls. This is because the symlink that we create is not in the same folder as the dlls.

@chrislovecnm chrislovecnm force-pushed the windows-symlink-pip branch from 76e1fe5 to 863dc7e Compare June 13, 2023 16:26
@chrislovecnm chrislovecnm changed the title feat(bzlmod): Fixing Windows Python Interpreter symlink issues fix(bzlmod): Fixing Windows Python Interpreter symlink issues Jun 13, 2023
@chrislovecnm chrislovecnm force-pushed the windows-symlink-pip branch 2 times, most recently from 81fa3a1 to 0f08282 Compare June 13, 2023 16:44
When using Windows you cannot run the symlink directly.

Because of how symlinks work in Windows we need to use the path
realink.
Unlike Linux and OSX we cannot execute the Python symlink directly.
Windows throws an error "-1073741515", when we execute the symlink
directory.  This error means that the Python binary cannot find dlls.
This is because the symlink that we create is not in the same folder
as the dlls.

This commit introduces code that resolves the actual location of the
interpreter using the path realink when we are running bzlmod and
Windows.
@chrislovecnm chrislovecnm force-pushed the windows-symlink-pip branch from 0f08282 to 98b51ab Compare June 13, 2023 16:51
Copy link
Collaborator

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just doc and ux improvements, otherwise lgtm

python/pip_install/pip_repository.bzl Outdated Show resolved Hide resolved
@@ -699,7 +708,7 @@ def _whl_library_impl(rctx):
)

if result.return_code:
fail("whl_library %s failed: %s (%s)" % (rctx.attr.name, result.stdout, result.stderr))
fail("whl_library %s failed: %s (%s) error code: '%s'" % (rctx.attr.name, result.stdout, result.stderr, result.return_code))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're here...

Suggested change
fail("whl_library %s failed: %s (%s) error code: '%s'" % (rctx.attr.name, result.stdout, result.stderr, result.return_code))
fail(("whl_library '{name}' failed: wheel_installer failed\n" +
"command: {args}\n" +
"error code: {error_code}\n" +
"===== stdout ====={stdout}\n" +
"===== stderr ====={stderr}\n=====").format(
name = rctx.attr.name,
command = args,
code=result.return_code,
stdout=result.stdout,
stderr=result.stderr
))

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test this code?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. i'm fine if you want to skip it for now.

Making English better.

Co-authored-by: Richard Levasseur <richardlev@gmail.com>
@rickeylev rickeylev enabled auto-merge June 13, 2023 18:55
@rickeylev rickeylev added this pull request to the merge queue Jun 13, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 13, 2023
@chrislovecnm chrislovecnm added this pull request to the merge queue Jun 13, 2023
@chrislovecnm chrislovecnm added the type: bzlmod bzlmod work label Jun 13, 2023
Merged via the queue into bazelbuild:main with commit 3903d1a Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bzlmod bzlmod work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants